Skip to content

Optimized for size Mathf.pow #1199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 21 commits into from
Apr 13, 2020
Merged

Optimized for size Mathf.pow #1199

merged 21 commits into from
Apr 13, 2020

Conversation

MaxGraey
Copy link
Member

No description provided.

@MaxGraey MaxGraey changed the title [WIP] optimized for size Mathf.pow Optimized for size Mathf.pow Mar 29, 2020
@MaxGraey MaxGraey requested a review from dcodeIO March 29, 2020 17:59
dcodeIO
dcodeIO reviewed Apr 3, 2020
View reviewed changes
@dcodeIO
Copy link
Member

dcodeIO commented Apr 3, 2020

Speaking of paths, do you know if we are testing all possible combinations? Like currently we instantiate and run untouched and fully optimized, but nothing in between, which might make us miss something for certain ASC_XY levels.

@MaxGraey
Copy link
Member Author

MaxGraey commented Apr 3, 2020

untouched usually cover ASC_SHRINK_LEVEL < X and optimized ASC_SHRINK_LEVEL >= X so I expect all condition compile paths covered

dcodeIO
dcodeIO approved these changes Apr 13, 2020
View reviewed changes
Copy link
Member

@dcodeIO dcodeIO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still believe we should somehow test more variations of optimization options, just to make sure we are not missing a branch, since if something goes wrong on a user's end these things will be exceptionally hard to track down. Probably fine to not have fixtures for these, so just compile and run the tests on them. From the comments it seems that this PR is not affected specifically by that, so LGTM.

@dcodeIO dcodeIO mentioned this pull request Apr 13, 2020
Closed
@dcodeIO dcodeIO merged commit e785a23 into AssemblyScript:master Apr 13, 2020
@MaxGraey MaxGraey deleted the new-mathf-pow branch April 13, 2020 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcodeIO dcodeIO dcodeIO approved these changes

Assignees
No one assigned
Labels
performance standard library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaxGraey @dcodeIO